Skip to content

Conversation

@CranberrySoup
Copy link
Contributor

@CranberrySoup CranberrySoup commented Oct 19, 2025

Changes the whole download system to:

  1. Add a user visible download queue
  2. Migrate away from WorkManager (since it does not work for everyone)
  3. Add instantaneous download button feedback
  4. Add batch downloading

Remaining issues to fix:

  • Batch download button (easy)
  • Fix the resume download button (easy)
  • Add link loading notification (easy)
  • Good download queue UI (hard)
  • Quality assurance

I plan to complete this pull request this week.

.data(url)
.apply {
headers?.forEach { (key, value) ->
extras[Extras.Key<String>(key)] = value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

headerBuilder["User-Agent"] = USER_AGENT

@fire-light42 fire-light42 mentioned this pull request Nov 10, 2025
5 tasks
@CranberrySoup CranberrySoup marked this pull request as ready for review December 6, 2025 20:53
@CranberrySoup
Copy link
Contributor Author

The queue should now work regardless of how you try to interrupt it. Please test it @fire-light42 @Luna712

Copy link
Contributor

@Luna712 Luna712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot_20251206_140424_CloudStream Debug

Haven't extensively tested much yet, but did find a UI bug with the download button in results.

@CranberrySoup
Copy link
Contributor Author

Thank you.
I did a lot of testing but only in series. I forgot the movies.

@CranberrySoup
Copy link
Contributor Author

Fixed 👍

Copy link
Contributor

@Luna712 Luna712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more issue, R.id.navigation_download_queue needs to be added to

when (destination.id) {
in listOf(R.id.navigation_downloads, R.id.navigation_download_child) -> {
navRailView.menu.findItem(R.id.navigation_downloads).isChecked = true

@Luna712
Copy link
Contributor

Luna712 commented Dec 7, 2025

Here is a bit more comprehensive review with more issues I've found. Though still not done testing completely.

  1. Managed to hit a crash:
Screenshot_20251206_182203_CloudStream Debug
  1. Cancel all queued downloads doesn't cancel in progress or paused downloads (not sure if intentional or not)
  2. Not sure if this is intentional either, but 1) you can't drag one from below the separator to above the separated to start that download immediately and requeue one above it. Additionally if you pause the 3 parallel downloads others don't start while those are paused. Though this may be intentional behavior.
  3. When I click the delete file on an in progress downloads while in the queue, it doesn't remove them from the adapter (or more likely view model because even if I close and reopen the fragment they still remain there).
  4. A bit inconsistent/odd padding for the download all button:
Screenshot_20251206_182838_CloudStream Debug

Perhaps the UI could be simplified by condensing into a download all icon? Not 100% sure though.

  1. When I click download all, the icons immediately change, but clicking them doesn't give an option to cancel them until it finishes finding the links for them. Also maybe a cancel all button should be added if all currently queued to reverse that download all action?
  2. Some animations randomly stop spinning. Not sure if that is just due to recyclerviews though:
    Screen_Recording_20251206_183220_CloudStream.Debug.2.mp4

@CranberrySoup
Copy link
Contributor Author

  1. Managed to hit a crash

It is such a weird issue because the mutex is surrounded by a try catch. I will try to fix it.

  1. Cancel all queued downloads doesn't cancel in progress or paused downloads (not sure if intentional or not)

This is intentional and I tried to clarify that using the wording in the popup. The queued downloads are instantaneous and easy to readd, making them of lower importance. While active downloads, which require link loading, are presumed to be more important, requiring explicit intent to cancel or pause.

  1. Not sure if this is intentional either, but 1) you can't drag one from below the separator to above the separated to start that download immediately and requeue one above it. Additionally if you pause the 3 parallel downloads others don't start while those are paused. Though this may be intentional behavior.

All of this behavior is intentional, although some of it may change in a future update. While it would be nice to move the separator or active downloads it would complicate the system, pull request and UX too much. Future pull requests may try to improve this, this pull request is big enough already.

  1. When I click the delete file on an in progress downloads while in the queue, it doesn't remove them from the adapter (or more likely view model because even if I close and reopen the fragment they still remain there).

I thought I fixed that issue, but I suppose not. Thank you for reporting it.

  1. A bit inconsistent/odd padding for the download all button. Perhaps the UI could be simplified by condensing into a download all icon? Not 100% sure though.

Will fix 👍
Netflix designs the button with an additional line in the download icon, maybe that would be nice to have.

  1. When I click download all, the icons immediately change, but clicking them doesn't give an option to cancel them until it finishes finding the links for them. Also maybe a cancel all button should be added if all currently queued to reverse that download all action?

I will add a click to cancel popup to each. However, I do not want to add more buttons to the result fragment. It already lacks space. A cancel all button in the queue menu is enough in my opinion.

Some animations randomly stop spinning. Not sure if that is just due to recyclerviews though.

I think the state is not properly set. I will try to fix.

Thank you for your review ❤️

@Luna712
Copy link
Contributor

Luna712 commented Dec 7, 2025

This is intentional and I tried to clarify that using the wording in the popup. The queued downloads are instantaneous and easy to readd, making them of lower importance. While active downloads, which require link loading, are presumed to be more important, requiring explicit intent to cancel or pause.

That makes sense and yeah I wasn't sure.

All of this behavior is intentional, although some of it may change in a future update. While it would be nice to move the separator or active downloads it would complicate the system, pull request and UX too much. Future pull requests may try to improve this, this pull request is big enough already.

Same as above

I will add a click to cancel popup to each. However, I do not want to add more buttons to the result fragment. It already lacks space. A cancel all button in the queue menu is enough in my opinion.

I wasn't necessarily thinking another button but rather replace the download all with cancel all if all in progress or delete all if all downloaded in that available batch, but the other button works too. Was just an idea, but isn't actually necessary for this PR. I think the first part of this I mentioned where you can't cancel at all until it finishes finding links is probably a bug though that could be fixed but not certain if that was intentional as well or not.

Thank you for your review ❤️

Of course, this PR really works amazing. Downloads no longer lag the entire app when in progress and the UI especially queue and instant download feedback is amazing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants